-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Replace println! with assert! if possible in DataFusion examples #11237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
please run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm thanks @Nishi46 I think we need to fix 1 example to make it happen.
Hope examples output is determenistic.
"Optimized Logical Plan:\n\n{}\n", | ||
logical_plan.display_indent() | ||
assert_eq!( | ||
logical_plan.display_indent().to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this actual plan is quite a bit more complicated than shown here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution @Nishi46
The examples in rewrite_expr.rs are being split up (see #11085) so in order to help this PR along, I pushed some commits to the relevant tests.
Once this PR is merged I think subsequent PRs will be easer to get in because CI will run automatically for you
If you want to keep helping out here (and I hope that you do!) perhaps you can make multiple PRs, one for each example? I think that will also help get the code in sooner rather than later
…che#11237) * Replace println! with assert! if possible in DataFusion examples * Replace println! with assert! if possible in DataFusion examples * Update rewrite_expr.rs * port changes to other examples --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
…che#11237) * Replace println! with assert! if possible in DataFusion examples * Replace println! with assert! if possible in DataFusion examples * Update rewrite_expr.rs * port changes to other examples --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Part of Closes #11230